-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Log to workdir #28
Log to workdir #28
Conversation
dfe7ff3
to
a654cb2
Compare
28780f4
to
a654cb2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor optimalization.
ansible_deploy/command_line.py
Outdated
logger.addHandler(file_handler) | ||
if log_dir: | ||
if not os.path.exists(log_dir): | ||
os.makedirs(log_dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
os.makedirs has a parameter: "exist_ok=bool" but I'm not sure what goes on low-level. In other case try/except would prolly be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well I'm not changing that, just moving things around :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it sounds better since the old approach is vulnerable to TOCTOU race
@@ -30,7 +30,7 @@ check_run_ok() { | |||
} | |||
|
|||
#Check wrong combinations | |||
check_output_fail 'ansible-deploy' '\[ERROR\]: Too few arguments' | |||
check_output_fail 'ansible-deploy' 'Too few arguments' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe some new log-level for early significant errors e.g. fatal.
[FATAL]: Too few arguments!
;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it may make sense, if we use it in logger for all the errors where we exit, but... logging doesn't have it. It has a critical which sounds like somthing we should us for critical errors already - diferent issue.
2bf644f
to
81a76b6
Compare
81a76b6
to
f261560
Compare
No description provided.